Skip to content

Conversation

@mcbarton
Copy link
Collaborator

Description

Please include a summary of changes, motivation and context for this PR.

There is a lot of back and forth in the build instructions in the readme for different platforms. To quickly get a set of instructions for either Windows or a Unix platform this PR arranges them into clickable sections when viewed on Github. e.g. it looks like this, where you click on the platform you want, to get the instructions just for that platform

image

To me this a lot cleaner, and easier for a beginner to decipher what exact instructions the user wants for their platform. To see how this looks in practice, and how it neatens everything up, see the branch of this PR https://github.com/mcbarton/CppInterOp/tree/Neaten-readme-build-instrucgtions

Fixes # (issue)

Type of change

Please tick all options which are relevant.

  • Bug fix
  • New feature
  • Requires documentation updates

Testing

Please describe the test(s) that you added and ran to verify your changes.

Checklist

  • I have read the contribution guide recently

@mcbarton mcbarton requested a review from vgvassilev July 11, 2025 18:18
@mcbarton
Copy link
Collaborator Author

Will work out how best to fix the linter issues next week (almost all about now having duplicate headers). Feel free to provide feedback on the idea of the PR in the meantime though. I will stop the workflows for now, and they can run when I fix the linter issues.

@mcbarton mcbarton force-pushed the Neaten-readme-build-instrucgtions branch 2 times, most recently from a0eddb2 to f21a206 Compare July 13, 2025 21:06
@codecov
Copy link

codecov bot commented Jul 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.52%. Comparing base (82f08c6) to head (c4b4da9).
⚠️ Report is 45 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #664   +/-   ##
=======================================
  Coverage   79.52%   79.52%           
=======================================
  Files           9        9           
  Lines        3917     3917           
=======================================
  Hits         3115     3115           
  Misses        802      802           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mcbarton
Copy link
Collaborator Author

Now that the markdown linter is passing for this PR it is ready for review.

Copy link
Collaborator

@Vipul-Cariappa Vipul-Cariappa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, will help beginners.
I was thinking; should we have nested expandable sections, for cling & clang-repl?

@mcbarton
Copy link
Collaborator Author

This looks good, will help beginners. I was thinking; should we have nested expandable sections, for cling & clang-repl?

Thanks for the feedback. I was thinking about this idea already, and will implement it sometime this week.

@mcbarton mcbarton force-pushed the Neaten-readme-build-instrucgtions branch 4 times, most recently from cadd5d5 to 8fec127 Compare July 21, 2025 13:26
@mcbarton mcbarton force-pushed the Neaten-readme-build-instrucgtions branch from f2c5826 to c4b4da9 Compare July 21, 2025 13:31
@mcbarton
Copy link
Collaborator Author

@Vipul-Cariappa I have applied your suggestion if you want to take another look.

@vgvassilev pinging for a review. This PR changes how the build instructions are navigated in the readme file (by using expandable sections). Go here https://github.com/mcbarton/CppInterOp/tree/Neaten-readme-build-instrucgtions to see what it would look like.

@mcbarton mcbarton requested a review from Vipul-Cariappa July 21, 2025 16:07
Copy link
Collaborator

@Vipul-Cariappa Vipul-Cariappa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.
I did not read the entire thing; Maybe we should specify somewhere that Clang-REPL is the preferred way to build CppInterOp, instead of Cling. @vgvassilev any thoughts?

@mcbarton mcbarton merged commit c13d6bd into compiler-research:main Jul 25, 2025
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants